Skip to content

Conversation

Weijun-H
Copy link
Contributor

@Weijun-H Weijun-H commented Oct 3, 2025

Related

Closes #10711

What

@Wumpf Wumpf requested a review from ntjohnson1 October 6, 2025 08:53
@ntjohnson1 ntjohnson1 added include in changelog examples Issues relating to the Rerun examples labels Oct 6, 2025
Copy link
Member

@ntjohnson1 ntjohnson1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you update the import in notebook_viewer to be from rerun.notebook instead of directly from our notebooks package? Otherwise LGTM. Thanks!

@Weijun-H Weijun-H requested a review from ntjohnson1 October 6, 2025 13:39
Copy link
Member

@ntjohnson1 ntjohnson1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the double review. Looking at the CI failure made me realize there were a few more issues but this should be it.

@Weijun-H Weijun-H force-pushed the 10711-move-notebook-example branch from b2acb22 to 9df19b1 Compare October 6, 2025 15:03
@ntjohnson1 ntjohnson1 merged commit bcb148f into rerun-io:main Oct 6, 2025
34 checks passed
ntjohnson1 added a commit that referenced this pull request Oct 8, 2025
### Related
#11416
I requested the notebooks get moved so our automated examples test would
make sure the notebooks aren't broken. Unfortunately we don't test any
of our notebooks so most were broken.

A little bit related to broader notebook testing improvement
https://linear.app/rerun/issue/RR-1880/test-our-jupyter-notebooks-on-ci

### What
I highly recommend reviewing with rich diff for notebooks enabled.
* Adds nbqa so we can run mypy on the notebooks to at least ensure the
apis align
* I couldn't get the config working so unfortunately I just have the
directories to search listed in the pixi command
* Adds nbstripout but doesn't yet tie it to our py-fmt. Jupyter notebook
outputs are just binary so that will bloat the repo and complicate
diffs. Unfortunately nbstripout doesn't have a nice config setup so I
just put together a simple wrapper.
* I made two actual code changes not just in tests since it made sense
to resolve notebook mypy errors
1. I made component_type a class method. We basically have component
type raw strings everywhere probably because it wasn't easy before to
grab them from the class definition rather than instance
2. If we have a recording string the api should actually be a little
different than asking globally. I assume we'll always find the id so we
universally return a string.
   
This still doesn't execute our notebooks which is probably a worthwhile
followup issue.
Wumpf pushed a commit that referenced this pull request Oct 8, 2025
### Related
#11416
I requested the notebooks get moved so our automated examples test would
make sure the notebooks aren't broken. Unfortunately we don't test any
of our notebooks so most were broken.

A little bit related to broader notebook testing improvement
https://linear.app/rerun/issue/RR-1880/test-our-jupyter-notebooks-on-ci

### What
I highly recommend reviewing with rich diff for notebooks enabled.
* Adds nbqa so we can run mypy on the notebooks to at least ensure the
apis align
* I couldn't get the config working so unfortunately I just have the
directories to search listed in the pixi command
* Adds nbstripout but doesn't yet tie it to our py-fmt. Jupyter notebook
outputs are just binary so that will bloat the repo and complicate
diffs. Unfortunately nbstripout doesn't have a nice config setup so I
just put together a simple wrapper.
* I made two actual code changes not just in tests since it made sense
to resolve notebook mypy errors
1. I made component_type a class method. We basically have component
type raw strings everywhere probably because it wasn't easy before to
grab them from the class definition rather than instance
2. If we have a recording string the api should actually be a little
different than asking globally. I assume we'll always find the id so we
universally return a string.
   
This still doesn't execute our notebooks which is probably a worthwhile
followup issue.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

examples Issues relating to the Rerun examples include in changelog

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Move Example Notebooks from rerun_notebooks to /examples

2 participants